Reorganise the devguide to directories #901
Conversation
|
All commit authors signed the Contributor License Agreement. |
CAM-Gerlach
requested review from
CAM-Gerlach,
encukou,
brettcannon and
ezio-melotti
and removed request for
pablogsal
|
FYI I have no opinion on this, so I'm dropping my review request. |
|
FYI, just in time for this PR, GitHub (finally) implemented the ability to seamlessly view commit history across file renames and moves. |
There was a problem hiding this comment.
Thanks again, @AA-Turner ! LGTM, aside from a few minor issues.
-
I grepped and found hardcoded external links to devguide.python.org three places (which are now outdated), which should be replaced with
:ref:and any links using them updated:-
core-developers/committing.rst:231:.. _`Python Triage Team` -
getting-started/getting-help.rst:29: `Discourse <https://devguide.python.org/communication/#discourse-discuss-python-org-web-forum>`_ -
triaging/labels.rst:413:.. _release schedule: https://devguide.python.org/#status-of-python-branches
-
-
Also, I'm still curious about the under/over header format used here—while an extra column on each side is included in some of the docutils examples, it is not what is specified in either the Sphinx documentation or the canonical Python docs style guide in this very repo, nor have I commonly seen it elsewhere...
| @@ -1,70 +0,0 @@ | |||
| Appendix: Topics | |||
There was a problem hiding this comment.
Is this file completely gone?
The file itself is probably redundant after this PR, but are the "Recommended readings" preserved somewhere?
There was a problem hiding this comment.
are the "Recommended readings" preserved somewhere?
No. Where would you suggest, if we should keep them?
A
There was a problem hiding this comment.
As @ezio-melotti suggests right below, they could go under the Audience categories (Contributor, Triager, Documentarian, CoreDev) in the restored table in the index. That would be a lot better than reverting whole removal and is the same or less work than reverting the removal since it would mean updating the doc links to ref links and fixing the target names, or fixing the outdated file paths and names, and also adding the pages that were added/split and now missing here or in the wrong sections.
There was a problem hiding this comment.
updating the doc links to ref links and fixing the target names, or fixing the outdated file paths and names, and also adding the pages that were added/split and now missing here or in the wrong sections.
I did these in a commit that I kept, so they should still work. Time-wise I probably don't have time to go through and update the table in the next week or so, so reverting was the quickest option.
A
There was a problem hiding this comment.
For the table (as far as I can tell), but not the appendix, at least as of the version you've pushed here—it still uses :doc: links, and almost all of them are now broken, as can be seen on the preview. Furthermore, it is missing the documents split as part of this PR, leaving it incomplete.
Instead of tediously updating all of that (or introducing a major regression), only to remove it again in a followup PR, I suggest just spending the same or less time re-dleeting it and adding them to the table instead as nessesary. Additionally, to keep it shorter and less work to update, we can just link the top-level index pages of sections that are obviously of central relevance to the audience type in their entirety, instead of linking every sub-page.
Here's what I suggest as the layout, and I'm happy to make a PR to your branch to implement it if you don't have time (which should take around the same or less as fixing the appendix):
Suggested table structure (by column heading)
All contributors:
- Getting Started
- Following Python's Development
- Issue Tracker
- Running and Writing Tests
Documentarians:
- Documentation
- Development Cycle
- Experts Index
- Triaging an Issue
Triagers:
- Issues and Triaging
- Development Workflow
- Experts Index
- Accepting Pull Requests
Core Developers:
- Core Developers
- Development Workflow
- Issues and Triaging
- Testing and Buildbots
|
I do have a concern on scope -- for instance title case / sentence case of headings. I would suggest to merge-as-is (once I've resolved the outstanding points) and do those sort of things as follow-ups. A |
|
Fixed references, increased max depth, removed spaces from titles, reverted line ending change in developers.csv. A |
|
I restored the appendix and topic table again to reduce scope of this PR as it seems removing them needs more discussion (Ezio's points). Partially I want to get this in as it puts this / other PRs in a state of limbo w.r.t. the moves. A |
There was a problem hiding this comment.
Minor note: The three remaining instances of the devguide links using external URLs were updated in the text, but the link targets were not actually removed. Searching for \.\. _\w+.*: .*devguide\.python should find all of them.
While I agree we shouldn't scope-creep this further and change the style of all the headings, I don't think we should rush this PR now without resolving the final issue with the appendix and index table being redundant/out of date, since:
- These are pretty central to the question of how to organize and structure the devguide (more so than some of the other implemented changes)
- This would result in a significant regression since the appendix is missing the newly split docs, has out of date file paths and is either out of sync with or duplicate much of the new structure, unless we do the work here to update it.
- There seems to be a fairly straightforward solution suggested by multiple people here, which would probably be around the same amount of work as updating the existing redundant appendix.
Also, keep in mind this can't be merged immediately until someone with the appropriate RTD permissions sets up the redirects (or we do them ourselves with my script or sphinx-redirects), and a repo admin can merge the PR (due to the CLA bot bug)— @ezio-melotti are you one here? And the existing PRs should transfer, so long as the move was detected (as most were) instead of the page split, and the relevant text wasn't changed.
As @ezio-melotti suggests, the various recommended reading relevant to each of the audience types can go under the respective table colums (Contributor, Triager, Documentarian, CoreDev) in the table on the index page index. That would be a lot better than reverting whole appendix removal and should be a similar amount of work as updating both to properly reflect the changes.
triaging/labels.rst
Outdated
| .. _GSoC: https://summerofcode.withgoogle.com/ | ||
| .. _issue tracker: https://devguide.python.org/tracker/ | ||
| .. _GitHub pull requests: https://github.com/python/cpython/pulls | ||
| .. _Python source code repositories: https://github.com/python/cpython/ | ||
| .. _Reporting security issues in Python: https://www.python.org/dev/security/ | ||
| .. _python-ideas: https://mail.python.org/mailman3/lists/python-ideas.python.org | ||
| .. _release schedule: https://devguide.python.org/#status-of-python-branches |
There was a problem hiding this comment.
| .. _release schedule: https://devguide.python.org/#status-of-python-branches |
This was fixed above and marked resolved but missed being removed here.
Some smallish benefits to having Sphinx-level redirects (either your script,
|
Apparently not, as I don't have the option to force-merge, nor access to the repo settings (I requested access in python/steering-council#128). |
|
I definitely agree; having them in the repo and generating them as part of the build process allows testing them locally, is portable between hosts and deployment mechanisms, and makes 100% sure they are in sync with the repo itself (particularly in this case, since otherwise a bunch of inbound links will break), among the other benefits you mentioned.
Of course, the RTD config should also be checked in to the repo, especially on one with many contributions/contributors and few admins with limited bandwidth; I've found in general that always checking in infra config and updating them in sync with PRs is far more maintainable than buried in the config of some external service asynchronous from the content changes and that only a few people have access to. |
|
Good news, the CLA bot is fixed! We can do a regular merge when we're ready! |
|
Here's an example of how to have HTML meta redirects without adding a new extension or such, within regular reStructuredText. https://raw.githubusercontent.com/pypa/pip/main/docs/html/reference/pip_cache.rst |
|
Given that all of the newly introduced "root" documents are basically just toctrees, I think an option worth considering is:
This would bring the navigation closer to something similar to https://tailwindcss.com/docs/. I haven't looked at the actual code changes, only the rendered documentation, but IMO this approach would also lend toward an easier-to-review PR -- since none of the files would move, none of the pages need updates to their references and the markup changes/cleanups can move into a dedicated follow up. It's also more incremental and likely slower, but it will make reviewing the code changes easier and also result in shorter PRs (which tend to not have as many things slip through the cracks as large reorganisation PRs). :) |
|
Done. A |
|
Merged, thanks @AA-Turner for the PR and to all the people who reviewed it! |
From the discord discussion, this is an attempt to add some structure to Docs/Tests/Triage and clean up the top level.
No content has been edited, except for combing two duplicated "how to become a triager" sections. As I moved content between pages I had to edit the underlines -- I also standardised some
^underlines and out-of-order underlines to allow easier page moves in the future (e.g. a core-dev folder, buildbots, language design and change).cc: @ezio-melotti @encukou @CAM-Gerlach
Preview: https://cpython-devguide--901.org.readthedocs.build/
Follows on from #895 where things went catastrophically wrong.
A